fix: Encode a01 values as json strings #645
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the A01 protocol encoding to match the old API behavior by encoding all values as JSON strings. Previously, values were passed through as-is; now they are wrapped with json.dumps() to ensure protocol compatibility.
- Applied
json.dumps()to all values in the dps dictionary withinencode_mqtt_payload() - Updated existing tests to expect JSON-stringified values
- Added a new test to verify list values are properly encoded as strings
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| roborock/protocols/a01_protocol.py | Modified encode_mqtt_payload() to apply json.dumps() to all values, converting them to JSON strings before encoding |
| tests/protocols/test_a01_protocol.py | Updated test expectations to reflect JSON string encoding, added imports for manual payload decoding, and added new test case for list conversion |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Dec 7, 2025
Merged
Collaborator
|
Test are failing |
Lash-L
previously approved these changes
Dec 7, 2025
Contributor
Author
|
I want to come back to this with better tests |
Update the a01 internal values to be json strings inside the dictionary. This matches the old API behavior.
allenporter
force-pushed
the
a01-fixes
branch
from
5e95ed7 to
89f0109
Compare
allenporter
commented
Dec 7, 2025
Contributor
Author
allenporter
left a comment
allenporter
left a comment
There was a problem hiding this comment.
OK, tests are not updated to catch the bug.
(The original change added double encoding to the old API and the encoding was not tested)
Lash-L
approved these changes
Dec 7, 2025
allenporter
added a commit
to allenporter/python-roborock
that referenced
this pull request
Dec 7, 2025
The bug was introduced in Python-roborock#645. Add tests that exercise the actual request encoding. This changes the ID QUERY value encoding by passing in a function, which is another variation on the first version of Python-roborock#645 where the json encoding happened inside the decode function.
allenporter
added a commit
that referenced
this pull request
Dec 8, 2025
* fix: Fix exception when sending dyad/zeo requests The bug was introduced in #645. Add tests that exercise the actual request encoding. This changes the ID QUERY value encoding by passing in a function, which is another variation on the first version of #645 where the json encoding happened inside the decode function. * chore: fix tests to be focused on value encoder * chore: fix lint
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update the a01 internal values to be json strings inside the dictionary. This matches the old API behavior.
Issue #623